Udev-related fixes and cleanups#279
Conversation
|
Hello @prajnoha I'm not saying that we are not going to merge this but it would take time before it is deeply validated. Did you tested it with exernal metadata? @bkucman could you please test this with IMSM? |
Yes, I have tested with IMSM (with |
Move the symlink wait to the common exit path, after udev is unblocked and the uevent is sent. The re-add loop does not need the symlink. Add a flag to distinguish successful array start from the "not enough disks" case to avoid spurious timeouts. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
Waiting for the symlink before sending the uevent causes a race: udev may see the initial add event before metadata is set, skip symlink creation, and cause a ~3.6s timeout. Move the symlink wait after the uevent so udev re-evaluates with correct metadata first. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
Sysfs rule application does not need the symlink. Move the symlink wait after it so it is the last step before return. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
Several error paths return without unblocking udev, leaking the blocking file on disk. Consolidate cleanup into a single error exit so all post-block failures unblock properly. Remove the redundant safety net in the caller now that create_mddev() handles its own cleanup. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
The release path sends a uevent unconditionally, but subarrays already get their own after assembly and error paths have nothing to signal. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
When a container member is skipped by the filter, udev is not unblocked, leaking the blocking file on disk. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
Add udev_ready() that combines udev_unblock() with sysfs_uevent() for the common "device is ready" path. This replaces the recurring pattern of calling both functions in sequence when an array has been fully set up and is ready for udev to re-evaluate. Call sites that only need udev_unblock() (error/abort/skip paths) or only sysfs_uevent() (container mid-path in Create) are left unchanged, as they represent distinct use cases. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
Move the udev_is_available() check from create_mddev() to its callers in Incremental.c and Create.c, letting them track in a local udev_blocked variable whether udev was actually blocked. All udev_unblock() and udev_ready() calls are then guarded by this flag, preventing spurious calls when udev is not available or when the code path never called create_mddev() (e.g. second disk arriving for an already-existing IMSM container). Add a diagnostic to udev_unblock() that warns if called without a prior udev_block(), catching any remaining misuse. Signed-off-by: Peter Rajnoha <prajnoha@redhat.com>
I'm more concerned about boot flows with OS installed on IMSM raid here is why I asked bkucman to verify (imsm is owned by graid now). |
|
Yes, this change requires testing on real test case scenarios, I’ll internally plan such tests on the hardware with IMSM. |
3 participants
This patchset cleans up udev blocking lifecycle management across Incremental, Create, and Assemble, fixing several bugs and a race condition along the way.
Problems addressed:
Symlink wait vs. uevent ordering race. In the Incremental container path, wait_for() ran before the change uevent was sent. udev could see the initial add event before metadata was set, skip symlink creation, and cause a ~3.6s timeout. The same misordering existed in the non-container path and in Assemble (relative to sysfs_rules_apply()).
Leaked udev blocking files. Several error paths in create_mddev() and Incremental_container() returned without calling udev_unblock(), leaving the blocking file on disk. Skipped container members also leaked the blocking file.
Redundant/spurious udev calls. udev_unblock() and sysfs_uevent() were called unconditionally even when udev was not available or when the code path never called udev_block() (e.g. a second disk arriving for an already-existing IMSM container). The container release path sent a redundant uevent that subarrays had already sent individually.